-
Notifications
You must be signed in to change notification settings - Fork 38.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove duplicate get errs #37270
Remove duplicate get errs #37270
Conversation
Can a kubernetes member verify that this patch is reasonable to test? If so, please reply with "@k8s-bot ok to test" on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands will still work. Regular contributors should join the org to skip this step.
If you have questions or suggestions related to this bot's behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
|
@fabianofranz for experience. |
@k8s-bot ok to test |
@fabianofranz PTAL |
/lgtm |
@ncdc needs a help adding |
You can lgtm, but not set a release-note? |
Correct, which makes my lgtm pretty much useless. Hopefully it's close to being addressed: kubernetes-retired/contrib#1908 (comment). |
@ncdc Thanks, also needs to remove |
actually @deads2k ^ |
@k8s-bot test this issue: #IGNORE Tests have been pending for 24 hours |
@k8s-bot test this issue: #IGNORE Tests have been pending for 24 hours |
@k8s-bot gci gce e2e test this |
@xilabao: you can't request testing unless you are a kubernetes member. In response to this comment:
If you have questions or suggestions related to this bot's behavior, please file an issue against the kubernetes/test-infra repository. |
@@ -464,6 +470,10 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ | |||
filteredResourceCount++ | |||
continue | |||
} | |||
if errs.Has(err.Error()) { | |||
continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This if statement changes behavior -- previously, errors from filterfuncs would not block printerobj handling.
I think you want:
if !errs.Has(err.Error()) {
errs.Insert(err.Error())
allErrs = append(allErrs, err)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, fixed
e1f74b7
to
0dc166a
Compare
@fabianofranz PTAL |
/lgtm |
Jenkins GCI GCE e2e failed for commit 0dc166a. Full PR test history. The magic incantation to run this job again is |
Jenkins GCE etcd3 e2e failed for commit 0dc166a. Full PR test history. The magic incantation to run this job again is |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue (batch tested with PRs 37270, 38309, 37568, 34554) |
old:
new:
This change is